Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Features page script #2503

Merged
merged 4 commits into from
Feb 24, 2022
Merged

Refactor Features page script #2503

merged 4 commits into from
Feb 24, 2022

Conversation

JakePT
Copy link
Contributor

@JakePT JakePT commented Dec 8, 2021

Description of the Change

This PR refactors the dashboard script and adjusts the markup to:

  • Remove the jQuery dependency.
  • Treat each feature as an individual <form> to take advantage of form APIs to simplify the script and markup, and improve keyboard accessibility.
  • Use correct elements for accessible toggles and buttons with appropriate aria attributes.
  • Remove vestigial unused code and dependencies.

It does not adjust the JavaScript for the ElasticPress > Settings page, so that has been moved into a separate file.

Benefits

Improves keyboard accessibility, by things such as allowing submitting the forms with the Enter key. It also simplifies the code so that it does not depend on custom data attributes and form handling.

Possible Drawbacks

Not necessarily a drawback, but the change results in a visual change for the Learn more and Collapse buttons:

Screen Shot 2021-12-08 at 10 48 15 pm

Verification Process

Go to ElasticPress > Features and adjust settings of all features and verify that they are saving correctly. Also check ElasicPress > Settings and verify that it functions as expected.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

Closes #2496

Changelog Entry

  • Improves accessibility of the Features screen.

@Rahmon
Copy link
Contributor

Rahmon commented Jan 13, 2022

@JakePT could you resolve the conflicts so I can review this PR?

@JakePT
Copy link
Contributor Author

JakePT commented Jan 14, 2022

@Rahmon Conflicts resolved.

@JakePT JakePT requested a review from felipeelia February 23, 2022 04:17
@felipeelia felipeelia self-assigned this Feb 23, 2022
@felipeelia
Copy link
Member

@JakePT the linter is complaining about a few things in the ES code, do you mind giving a look into it? Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use form and button elements for Feature settings forms, remove jQuery dependency from related JavaScript
4 participants